Skip to content

Improved Update Folder behaviour#6575

Closed
Xemorr wants to merge 6 commits into
PaperMC:masterfrom
Xemorr:master
Closed

Improved Update Folder behaviour#6575
Xemorr wants to merge 6 commits into
PaperMC:masterfrom
Xemorr:master

Conversation

@Xemorr
Copy link
Copy Markdown
Contributor

@Xemorr Xemorr commented Sep 7, 2021

This patch modifies the checkUpdate method to replace based on plugin name instead of file name. This allows you to retain versioning in your plugin's file names and use the update folder. When the plugin is replaced with the one in the update folder, the name of the plugin in the update folder is kept as well.

This closes the feature request I opened #6570

If you have any suggestions on how I can improve this patch, feel free to tell me, I'm open to criticism :)

EDIT: I have now been running this on my servers for a couple of days now, this is a linux machine, so it's tested on Windows, Mac OS & Linux now and it's been working a treat for me.

… name rather than the file name. The name of the file after the replacement is the name of the file in the update folder.
@Xemorr Xemorr requested review from a team as code owners September 7, 2021 19:19
@NoahvdAa
Copy link
Copy Markdown
Member

NoahvdAa commented Sep 7, 2021

FYI, Multi line changes start with // Paper start and end with // Paper end (// Paper for singe lines)

@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Sep 7, 2021

oh yeah forgot to do that

@NoahvdAa
Copy link
Copy Markdown
Member

NoahvdAa commented Sep 7, 2021

Tried to update ViaVersion 3.2.1 to 4.0.1 with this, the server seems to crash.

crash-2021-09-07_22.27.17-server.txt

@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Sep 7, 2021

oh that's odd, I tested it with my plugins and it worked fine. I'll investigate that

@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Sep 7, 2021

Are you sure they had .jar file extensions on the end?

@NoahvdAa
Copy link
Copy Markdown
Member

NoahvdAa commented Sep 7, 2021

yes

@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Sep 7, 2021

Okay, I managed to update between those two viaversion versions fine on my machine, but I'm wondering if it's an operating system thing as I know file code can break between operating systems and you're on MacOS and I've been testing on Windows.

@NoahvdAa
Copy link
Copy Markdown
Member

NoahvdAa commented Sep 7, 2021

Did some debugging, seems to be caused by macos file system junk, it was trying to update a .DS_Store file. Probably a good idea to ignore non .jar files.

@NoahvdAa
Copy link
Copy Markdown
Member

NoahvdAa commented Sep 7, 2021

Worked when ignoring non-java files by changing

if (updateFile.isFile()) {

to

if (updateFile.isFile() && updateFile.getName().toLowerCase().endsWith(".jar")) {

(probably not the cleanest solution)

EDIT: Definitely not the cleanest solution, solution below is better.

@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Sep 7, 2021

I think a simple if (updatePluginLoader == null) continue; would be better here as the getPluginLoader already returns a plugin loader if the file extension matches and if it doesn't match then it returns null so makes sense. Thanks for debugging that for me!

Comment thread patches/api/0332-Update-Folder-Uses-Plugin-Name.patch Outdated
Copy link
Copy Markdown
Member

@NoahvdAa NoahvdAa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works properly for me now!

Comment thread patches/api/0332-Update-Folder-Uses-Plugin-Name.patch
Comment thread patches/api/0332-Update-Folder-Uses-Plugin-Name.patch Outdated
@stale
Copy link
Copy Markdown

stale Bot commented Nov 16, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Nov 16, 2021 via email

@stale stale Bot removed the resolution: stale label Nov 16, 2021
@stale
Copy link
Copy Markdown

stale Bot commented Jan 15, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link
Copy Markdown

stale Bot commented Mar 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Chew
Copy link
Copy Markdown
Contributor

Chew commented Mar 18, 2022

We know what that means, rebase time 😄

@stale stale Bot removed the resolution: stale label Mar 18, 2022
@Xemorr
Copy link
Copy Markdown
Contributor Author

Xemorr commented Mar 19, 2022

Yeah I'll get on it in a couple days, it's a bit annoying it didn't get merged in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants